Skip to content

uno: zeroize per-IO DMA buffers on teardown using the GDMA engine - #632

Open
Rajib Dutta (radutta99) wants to merge 7 commits into
mainfrom
user/radutta/io-buf-zeroize-gdma
Open

uno: zeroize per-IO DMA buffers on teardown using the GDMA engine#632
Rajib Dutta (radutta99) wants to merge 7 commits into
mainfrom
user/radutta/io-buf-zeroize-gdma

Conversation

@radutta99

Copy link
Copy Markdown
Contributor

What

Uno never wiped its per-IO scratch. std zeroizes on buf_pool::free, but uno only rewound the bump watermark, so key material — RSA private keys, the P-384 identity scalar, OAEP plaintext — stayed resident in the per-IO SRAM/DTCM heaps and was visible to whichever IO reused the slot. Uno has no heap, so those bump buffers are the only place such material exists.

This adds a teardown scrub driven by the GDMA engine, per Jayant Gandhi (@jaygmsft)'s review comment on #602.

How

The wipe is a DMA copy from a read-as-zero window. GDMA has no fill/memset opcode — an SQ entry's OP_CODE only encodes read-src / write-dst — so zeroing means copying out of the SoC's "Dummy Memory" region at 0xA0B0_0000. That region wasn't modelled anywhere in the firmware, so it is now an RDL entry (dummy_mem.rdl) with the base and size generated into the reg crate rather than hardcoded.

Every host IO. The scrub hangs off drop_io, which the core dispatch loop calls on both the completed and the dropped paths.

Internal provisioning too. The admin slot never passes through drop_io, so identity/enable-time keygen and boot-key masking now run inside a with_admin_io session that scrubs on exit — obtaining an admin IO is entering a scrubbed scope. UnoHsmIo::admin_no_scrub is the one deliberate opt-out, named so a new call site has to choose it.

Only what was written. A per-(slot, heap) high-water mark bounds the wipe. dma_alloc_var* reserve the whole remaining heap so the callback need not know its encoded length up front; they snapshot the peak before reserving and restore it to the trimmed length afterwards, and document the contract that rests on. On the error path the peak stays at the full reservation, since a partial write before a failure is unbounded.

Hardware validation

Flashed to the Manticore EVB and instrumented to scan each slot's whole capacity after every scrub, classifying any non-zero byte as inside or beyond the wiped region:

  • No byte past the recorded peak, ever — across ~1.7k IOs on every slot. This is the premise of wiping a prefix, and it also confirms the dma_alloc_var contract holds for both the MBOR encoder and the TBOR frame builders.
  • No residue inside the wiped region, and no CPU fallback — GDMA handled every wipe.
  • One representative line: a slot wiped only 288 B, and the full 18432 + 1536 B read back zero.

Cost: the DMA peak averages ~933 B of the 18 KiB slot, max ~3.3 KiB — so the wipe fits in a single DUMMY_MEM-sized GDMA transfer instead of two.

Regression: 45/45 over three passes (cert-chain incl. multithread, ECC-sign compat across P-256/384/521, sign/verify, part-info). rsa_unwrap and attest_key match this branch's baseline exactly; their failures are pre-existing bring-up gaps (UnsupportedCmd for the PKA-stubbed ECC/RSA attestation, FileHandleNoExistingSession for NoSession routing).

Notes for review

  • io_peak starts at zero, so each slot's first scrub covers only what that boot wrote. That relies on the IO heaps arriving clean, which 1SP guarantees on a warm boot; the field doc records the dependency and what to change if that ever stops holding.
  • The DTCM heap is not on the GDMA fabric, so it is wiped by the CPU — word-wide, and with volatile writes that cannot be elided.
  • A GDMA failure falls back to a CPU wipe and is logged through the trace facade, so a persistent fault cannot silently degrade into an unsignalled byte loop.

Merge order

#602 removes its local zeroize calls on the assumption this lands, so if #602 merges first there is a window with no wipe on those paths. Worth sequencing this one first.

Rajib Dutta (radutta99) and others added 4 commits August 5, 2026 01:22
Internal provisioning (partition identity, enable-time keygen, boot-key
masking, vault teardown) runs on the reserved admin IO slot rather than a
host IO, so it never reaches drop_io. Its scratch was only watermark-
rewound, never wiped, leaving raw private-key material (e.g. the P-384
identity scalar) resident in the slot indefinitely.

Add UnoHsmPal::with_admin_io, the admin-side counterpart of drop_io: it
opens an admin IO plus a rewound scoped allocator, runs the caller's async
closure, then scrubs the slot. Binding the scrub to the session rather than
to each caller means no admin path can forget it, and the higher-ranked
bound stops any scratch from escaping past the scrub.

ensure_unwrapping_key_imported is left as-is: it is a synchronous fn whose
atomicity against the cooperative scheduler is load-bearing, and it feeds
the &'static GSRAM blob straight into the vault with no DMA-scratch copy.

Also fix a clippy manual_is_multiple_of warning in cpu_zeroize.
`with_admin_io`'s higher-ranked bound stops the closure returning anything
borrowed from the session's scoped allocator, and the doc claimed that meant
no scratch could outlive the scrub. That was not quite true: the closure
captures the PAL, and the PAL-level `dma_alloc` hands back a buffer whose
lifetime is tied to the PAL, not to the session. A closure could return one
and the caller would silently receive memory the scrub had just zeroed.

Add `R: 'static`. Every caller already returns an owned value, so this costs
nothing, and it turns the doc claim into something the compiler checks: an
attempt to return a PAL-level buffer out of the session now fails to compile
with "returning this value requires that `'1` must outlive `'static`".

Also record why `ensure_unwrapping_key_imported` still takes a raw admin IO
instead of a session: it is synchronous, so it cannot await a scrub, and it
allocates no scratch — `create_sync` copies from the `&'static` GSRAM slot
straight into vault storage — so there is nothing to wipe. Without the note
it reads like an omission.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
The teardown scrub wiped each slot's full capacity — 18 KiB of SRAM and
1.5 KiB of DTCM — however little the command touched. The live bump
watermark cannot bound that, because it rewinds on every scope exit, so
track a per-(slot, heap) peak that only grows and wipe to that instead.

The peak is cleared by the scrub rather than by the allocator reset, so it
always means "bytes written since the last scrub". Everything past it is
already zero from that scrub, and an IO that ends without one keeps its
peak, so the next scrub still covers it.

`dma_alloc_var*` need care: they reserve the whole remaining heap so the
callback need not know its encoded length up front, which would pin the
peak at capacity on every IO. They now snapshot the peak before the
reservation and restore it to the trimmed length, and document the contract
this rests on — the callback must not write past the length it returns. On
the error path the peak is left at the full reservation, since a partial
write before the failure is unbounded.

Measured on hardware over ~1.7k IOs: the DMA peak averages ~933 B of the
18 KiB slot with a maximum of ~3.3 KiB, so the wipe now fits in a single
DUMMY_MEM-sized GDMA transfer instead of two. Instrumenting the scrub to
scan each slot's whole capacity afterwards found no non-zero byte past the
recorded peak, on any slot, in any run.

Also in this change:

- `zeroize_mem` wipes through the GDMA engine rather than the CPU, falling
  back to a volatile CPU wipe if the engine has no free tag. Every `DmaBuf`
  is GSRAM-backed by construction — the DTCM heap hands out plain
  `&mut [u8]` — so the destination is always GDMA-reachable.

- `UnoHsmIo::admin` becomes `admin_no_scrub`, so a path that opts out of the
  scrubbed session has to say so. Its one caller is the synchronous
  unwrapping-key import, which cannot await a scrub and writes nothing to
  either bump heap.

- `delete_key` takes the caller's admin IO instead of opening a session per
  key. `KeyVault::delete` takes no allocator, so it cannot dirty the heaps,
  and `clear_enabled_state` deletes one key per provisioning slot and per
  live session — a session each would have scrubbed the slot every time for
  no benefit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
Copilot AI lite review requested due to automatic review settings August 6, 2026 01:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds reliable zeroization of Uno’s per-IO scratch heaps on IO teardown, primarily using the GDMA engine by copying from a read-as-zero “Dummy Memory” window, and extends the same scrubbing guarantees to internal/admin provisioning flows that don’t naturally pass through host IO teardown.

Changes:

  • Model the SoC “Dummy Memory” read-as-zero region in RDL/reggen and use it as the GDMA zero source for SRAM wipes.
  • Add per-IO high-water tracking and scrub both SRAM (via GDMA, CPU fallback) and DTCM (CPU volatile wipe) on drop_io and admin-session exit.
  • Refactor admin provisioning paths to run under with_admin_io(...) so the admin IO slot is scrubbed consistently (with a deliberate admin_no_scrub opt-out).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
fw/plat/uno/rdl/soc/uno.rdl Adds Dummy Memory region to the Uno SoC map and includes its RDL.
fw/plat/uno/rdl/soc/dummy_mem.rdl New RDL defining the read-as-zero Dummy Memory window used as GDMA wipe source.
fw/plat/uno/fw/reg/soc/src/lib.rs Exposes the generated dummy_mem register module.
fw/plat/uno/fw/reg/soc/src/dummy_mem.rs New autogenerated constants/regs for Dummy Memory base/size.
fw/plat/uno/fw/pal/src/part.rs Routes admin/provisioning flows through with_admin_io to ensure admin-slot scrubbing; introduces admin_no_scrub usage for the synchronous path.
fw/plat/uno/fw/pal/src/pal.rs Adds io_peak tracking alongside io_alloc to bound teardown scrubs.
fw/plat/uno/fw/pal/src/io.rs Scrubs per-IO scratch on drop_io; introduces with_admin_io and renames admin IO constructor to admin_no_scrub.
fw/plat/uno/fw/pal/src/gdma.rs Implements GDMA-based zeroization via Dummy Memory copy, adds scrub logic + CPU volatile fallback.
fw/plat/uno/fw/pal/src/alloc.rs Adds peak-watermark tracking and helpers to compute per-slot dirty prefixes; adjusts dma_alloc_var* to preserve accurate peaks.
Suppressed comments (1)

fw/plat/uno/fw/pal/src/alloc.rs:559

  • Same issue as dma_alloc_var: len is clamped for watermark/peak accounting, but &mut buf[..len] is not clamped and can panic if f returns a larger value than the reserved buffer length. Clamp the returned slice length as well.
                let end = start + len.min(buf.len());
                w.with(|v| *v = end);
                pk(self, io_index, DMA).with(|v| *v = peak_before.max(end));
                // SAFETY: `buf` came from the SRAM Dma pool.
                Ok((unsafe { DmaBuf::from_raw_mut(&mut buf[..len]) }, extra))

Comment thread fw/plat/uno/fw/pal/src/alloc.rs Outdated
`dma_alloc_var*` clamped the callback's `len` when advancing the watermark
but then sliced `buf[..len]` unclamped, so the two disagreed and a callback
that reported more than it was handed would panic — taking the firmware down
rather than failing the one command.

Reject it with `InvalidArg` instead, as the std PAL already does: refuse to
hand back a longer slice than we own. The watermark rewinds, but the peak
stays at the full reservation, since a callback that over-reports wrote an
unknown amount and the teardown scrub has to cover it.

Reported by the Copilot reviewer on #632.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
Copilot AI review requested due to automatic review settings August 6, 2026 01:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

fw/plat/uno/fw/pal/src/gdma.rs:250

  • gdma_zero_region chunks the transfer up to DUMMY_MEM_SIZE (16 KiB), but the GDMA driver documents inline SGL Data Block descriptors as max 4 KiB per transfer (fw/plat/uno/fw/drivers/gdma/src/types.rs). If an IO’s dirty region ever exceeds 4 KiB (e.g., the error paths that intentionally leave io_peak at the full reservation), the first 16 KiB copy is likely to fail and force the CPU fallback + warning on every such teardown. Limit each GDMA copy to 4 KiB (and optionally still cap by DUMMY_MEM_SIZE).
    /// Copies zeros from the [`DUMMY_MEM_BASE`] window, chunked to its
    /// [`DUMMY_MEM_SIZE`] so an arbitrarily large region can be wiped
    /// with a fixed 16 KiB zero source. Both operands use the device
    /// interface. Only valid for GDMA-reachable memory (GSRAM); the M7 TCM
    /// is not on the GDMA fabric and must be wiped by the CPU.
    async fn gdma_zero_region(&self, dst_ptr: *mut u8, len: usize) -> HsmResult<()> {
        let mut off = 0usize;
        while off < len {
            let chunk = core::cmp::min(len - off, DUMMY_MEM_SIZE as usize);
            let src = device_dma_buf(DUMMY_MEM_BASE as *const u8, chunk as u32);

`InvalidArg` was the wrong status for a callback that reports writing more
than the buffer it was handed. The enum's own docs reserve `InvalidArg` for
a caller bug — "unknown id, kind mismatch", "a malformed request" — but here
the request is fine and the fault is ours: an encoder mis-reporting its
output length. Returning `InvalidArg` blames the host for a firmware bug.

Add `DmaAllocLenOverrun`, following the `UndoLogFull` precedent of a
dedicated, documented status for a should-never-happen firmware bug, and use
it from both PALs so they report the same thing for the same condition.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
Copilot AI review requested due to automatic review settings August 6, 2026 02:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comment thread fw/plat/uno/fw/pal/src/gdma.rs
Comment thread fw/plat/uno/fw/pal/src/gdma.rs Outdated
//! | `IO_META[index]` | 8B metadata | Controller/queue IDs from IIC recv |
//! | `DTCM_IO_BUF[index]` | 1.5KB fmem | Fast DTCM workspace buffer |
//! | `SRAM_IO_BUF[index]` | 8KB smem | Large SRAM workspace buffer |
//! | `SRAM_IO_BUF[index]` | 18KB smem | Large SRAM workspace buffer |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you verify that the 18KB is in error and should be 8KB instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will wait for Vishal Soni (@vsonims) on this question. On my local testing with enabled DDI tests, 8KB uno works just fine.
Also note that as per the changes in this PR, zeroization performance does NOT depend on this size. It depends on how much size the IO end up making dirty in its lifetime. So we can leave the size as is today with no efficiency loss.

Copilot AI review requested due to automatic review settings August 7, 2026 23:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Addresses review feedback: use a vetted zeroization primitive and widen
the CPU store from 32-bit to 64-bit.

`cpu_zeroize` was a hand-rolled 32-bit volatile-store loop. Replace it
with the audited `zeroize` crate applied to a `[u64]` view of the region:
an 8-byte-aligned QWORD body framed by a byte-wise head/tail for any
unaligned edges. Both per-IO regions (`DTCM_IO_BUF` 0x600, `SRAM_IO_BUF`
0x4800) are 8-byte aligned and 8-multiples, so in practice it is a pure
`u64` wipe with no head or tail.

On Cortex-M7 `[u64]::zeroize()` lowers to `STRD` (64-bit doubleword
stores), verified in the release disassembly — the wipe loop is a 4x
unrolled `strd rZero, rZero, [ptr, #off]`. That halves the store count
versus the previous word loop and quarters a byte-wise wipe, on the CPU
fallback and the small DTCM path (the 18 KiB SRAM slot is still wiped by
GDMA). `zeroize` emits the volatile writes plus an atomic fence, so the
wipe still cannot be elided.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2444329e-909e-4ced-b5df-5a40fb65bb6c
Copilot AI review requested due to automatic review settings August 10, 2026 17:27
@radutta99
Rajib Dutta (radutta99) force-pushed the user/radutta/io-buf-zeroize-gdma branch from 7dbbe27 to 443ccf9 Compare August 10, 2026 17:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (1)

fw/plat/uno/fw/pal/src/gdma.rs:307

  • scrub_io_slot logs a warning on any GDMA failure, including expected transient resource exhaustion (e.g. GdmaError::NO_FREE_TAGS / SQ_FULL). In a busy system this can spam logs and make it harder to notice real scrub regressions; it also loses the actual error value by using .is_err().

Capture the GDMA error and only warn! on unexpected failures (still falling back to the CPU wipe for all errors).

        let (dma_ptr, dma_len) = crate::alloc::io_slot_dma_dirty(self, io_index);
        if dma_len != 0 && self.gdma_zero_region(dma_ptr, dma_len).await.is_err() {
            // Surface the fallback: a persistent GDMA fault would otherwise
            // silently degrade every wipe into a CPU loop with no signal.
            // Logged via the trace facade (as the iic/oic drivers do for

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants